Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLIENT-SPECIFICATION: clarify fallback to english #4101

Merged
merged 14 commits into from
Jul 21, 2020

Conversation

sbrl
Copy link
Member

@sbrl sbrl commented Jun 11, 2020

As per the conversation in #4092, I've opened this PR.

I've changed the wording again a bit in order to comply with RFC 2119 and RFC 6919.

@sbrl sbrl added documentation Issues/PRs modifying the documentation. clients Issues pertaining to a particular client or the clients as whole. labels Jun 11, 2020
CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
@MasterOdin
Copy link
Collaborator

Pulling my last comment #4092 (comment) here:

My intention with the first sentence was to mandate the fallback to English, and would advocate heavily for upgrading "SHOULD" to "MUST" (I realize I used should in my original draft, apologies).

I think there are already too many optional bits to the language spec, and I think this one should be mandatory as it provides a consistent behavior across clients that do language (namely that they fallback to English).

command line -> command-line when used as an adjective
@zlatanvasovic
Copy link
Contributor

@sbrl I edited two minor things. Other than that, the proposal is all good.

@columbarius
Copy link

I was suggested to engage in this discussion. Since this PR is about the specific formulation of the specification I added my suggestion to the #4092 (comment).

@zlatanvasovic
Copy link
Contributor

This needs review by org owners, i.e. @agnivade @owenvoke @mebeim.

@sbrl
Copy link
Member Author

sbrl commented Jun 21, 2020

@MasterOdin I completely agree about the fallback there. I'll edit.

@agnivade
Copy link
Member

I haven't been following this discussion closely. I'll settle for whatever is decided here.

Co-authored-by: Matthew Peveler <[email protected]>
CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
Copy link
Member

@mebeim mebeim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check out the changes below @sbrl. Plus, I would suggest using @MasterOdin's suggestion since that seems the correct semantic for the LANGUAGE env var, also adding a bullet point for this in the changelog.

@sbrl
Copy link
Member Author

sbrl commented Jun 27, 2020

Updates complete!

CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
@mebeim
Copy link
Member

mebeim commented Jun 27, 2020

Re-reading the env var paragraph, I don't think that explicitly requiring LANG to be set is needed here. The linked GNU gettext reference does not actually say that LANGUAGE is to be ignored if LANG is not set.

This paragraph is quite complex. I'll try to reword it when I have time to make it as simple and clear as possible so that you guys can tell me what you think, but in a nutshell I think we should just stick to the priority list of this documentation page, simply giving LANGUAGE priority over LANG. Ignoring LANGUAGE if LANG is not set is just a missed chance to follow the user's preference.

@columbarius
Copy link

columbarius commented Jun 27, 2020

Re-reading the env var paragraph, I don't think that explicitly requiring LANG to be set is needed here. The linked GNU gettext reference does not actually say that LANGUAGE is to be ignored if LANG is not set.

As i read the Note:-Part LANG must be present to enable the LANGUAGE variable. I tested it with gettext --usage and found exactly this behaviour:

% echo $LANG                          
de_DE.UTF-8
% gettext --usage
gettext: Unbekannte Option »--usage«
»gettext --help« gibt weitere Informationen.
% LANGUAGE=it_IT:de_DE gettext --usage
gettext: Unbekannte Option »--usage«
Usare 'gettext --help' per maggiori informazioni.
% env -u LANG LANGUAGE=it_IT:de_DE gettext --usage
gettext: unrecognized option '--usage'
Try 'gettext --help' for more information.
% LANG=en gettext --version
gettext (GNU gettext-runtime) 0.20.2
Copyright (C) 1995-2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Written by Ulrich Drepper.

You may try this on your system.
Edit: Fail on my side, i didn't noticed, that gettext is missing a usage option, but help prints much more text and for demonstration purpose this should be enough.

@mebeim
Copy link
Member

mebeim commented Jun 28, 2020

@columbarius Ok I see. Then the bottom line is: respect $LANG when available, followed by the list in $LANGUAGE if present, otherwise use English. I still believe the current paragraph is quite complex so I'll reword it.

@sbrl
Copy link
Member Author

sbrl commented Jul 2, 2020

Thanks for spotting the typo @mebeim! I guess I'll hold off on merging until you've re-worded it?

@columbarius
Copy link

@mebeim Yes, respect $LANG when available, followed by the list in $LANGUAGE which prioritizes the value in $LANG, which is the confusing part of this standard. If $LANG and $LANGUAGE is set, respect$LANGUAGE before $LANG, but if $LANG is not set, ignore $LANGUAGE.

My example with the gettext command, shows, that this is used for stdout, but stderr just uses $LANG and ignores $LANGUAGE, but I don't think, we have to use that behaviour.

Copy link
Member

@mebeim mebeim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for holding this back! As promised, I've tweaked the wording and reordered paragraphs to make the Language section clearer. See here for an easily readable version. It's fine to merge for me, if anyone has any comment and/or correction then please do let me know.

CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
Co-authored-by: Starbeamrainbowlabs <[email protected]>
@zlatanvasovic
Copy link
Contributor

Since virtually every team member agreed with this, I guess we can merge it now?

@sbrl
Copy link
Member Author

sbrl commented Jul 21, 2020

Sounds good to me @zdroid - go ahead :D

@zlatanvasovic zlatanvasovic merged commit 8effb5b into master Jul 21, 2020
@zlatanvasovic zlatanvasovic deleted the spec/english-fallback branch July 21, 2020 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clients Issues pertaining to a particular client or the clients as whole. documentation Issues/PRs modifying the documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants